Fix Cassandra case insensitive issue #2897
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @KodaiD, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request resolves a long-standing bug in the Cassandra administration component that prevented successful table alterations when table names exhibited case differences. By ensuring proper quoting of identifiers, the system now correctly handles case-sensitive table names, significantly improving the robustness of schema management operations. The changes also include targeted test enhancements to verify the fix.
Highlights
- Cassandra Case Sensitivity Fix: I've addressed a critical issue in
CassandraAdminwhere altering tables failed due to case insensitivity. The fix involves properly quoting namespace and table names when executingALTER TABLEstatements to preserve their original casing. - Refinement of Quoting Logic: I've also refined the usage of
quoteIfNecessarywithinCassandraAdminby removing unnecessary quoting for internal metadata operations (e.g., when interacting with theNAMESPACES_TABLE), ensuring that quoting is applied only where required for user-defined schema elements. - Enhanced Integration Tests: To validate the fix and prevent future regressions, I've updated the integration tests to use case-sensitive namespace and table names, providing better coverage for this specific scenario.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request addresses a case-sensitivity issue in CassandraAdmin when altering tables. The main fix involves quoting the namespace and table name in ALTER TABLE statements, which is correct. The PR also cleans up incorrect quoting of values in other queries, which improves correctness. The integration tests are rightly updated with mixed-case identifiers to cover the fixed scenario. I've identified one potential issue where a column name in the same ALTER TABLE statement is not being quoted, which could lead to similar case-sensitivity problems for column names.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a case sensitivity issue in CassandraAdmin where altering tables would fail when table names contained mixed case characters. The fix ensures proper quoting of table and namespace names during ALTER TABLE operations while removing unnecessary quoting from metadata operations.
- Applied
quoteIfNecessaryto table and namespace names inaddNewColumnToTablemethod - Removed unnecessary
quoteIfNecessarycalls from metadata table operations for column values - Updated integration test constants to use mixed-case names for better test coverage
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java | Fixed quoting logic for ALTER TABLE operations and metadata operations |
| core/src/test/java/com/scalar/db/storage/cassandra/CassandraAdminTest.java | Updated test expectations to match the fixed quoting behavior |
| integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java | Changed test constants to mixed-case names to verify case sensitivity handling |
KodaiD
requested review from
a team,
Torch3333,
brfrn169,
feeblefakie and
komamitsu
and removed request for
a team
...gration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/scalar/db/storage/cassandra/CassandraAdmin.java
Outdated
Show resolved
Hide resolved
...gration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java
Outdated
Show resolved
Hide resolved
5 participants
Description
This PR fixes an issue where
CassandraAdminfails to alter a table due to case insensitivity in the object name.The current implementation of
CassandraAdmindoes not handle case sensitivity correctly when altering a table, leading to a not found error. For example, if a table is created with the nameTest_table, attempting to alter it with the nametest_tablewill result in an error because the table name is not found.The fix is to enclose the namespace/table/column name in
""when altering the table, ensuring that the case is preserved. Also, this PR updated the integration tests to cover the case.Related issues and/or PRs
N/A
Changes made
quoteIfNecessarywhen altering a table inCassandraAdmin.quoteIfNecessarywhere it is not necessary inCassandraAdmin.Checklist
Additional notes (optional)
N/A
Release notes
N/A